Repository navigation
Conversation
Contributor
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
4 open issues
kernel_not_supported, but the message still reads like a… · Newerrc::kernel_not_supportedis used unqualified here while other SYCL types are qualified… · Newhandler-path coverage forCGH.copy(...), but the PR also changes behavior for… · New
What changed in this PR
Updates SYCL device_global copy/memcpy behavior to comply with the device_image_scope requirement: copying to/from a device_global that is not referenced by any kernel in its device image now throws errc::kernel_not_supported.
Changes:
- Add unit and e2e coverage for
kernel_not_supportedwhen copying/memcpy’ing an unuseddevice_image_scopedevice_global - Change runtime behavior in
handlerto throw (instead of host-only fallback) for unuseddevice_image_scopedevice_global - Align a related runtime error code in
memory_managertoerrc::kernel_not_supported
| File | Description |
|---|---|
| sycl/unittests/Extensions/DeviceGlobal.cpp | Adds unit tests for throwing behavior with unused device_image_scope, and validates host-only behavior for non-scoped globals |
| sycl/test-e2e/DeviceGlobal/device_global_unused_dis.cpp | Updates e2e test to expect kernel_not_supported for unused device_image_scope globals |
| sycl/source/detail/memory_manager.cpp | Changes error code to kernel_not_supported when no device image contains the device_global |
| sycl/include/sycl/handler.hpp | Throws kernel_not_supported for unused device_image_scope globals; keeps host-only fallback for non-scoped globals |
Review effort: Lite
Copilot stopped reviewing on behalf of
dyniols due to an error
November 3, 2026 15:44
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues were identified for the updated handling of copies involving unused image-scoped device_global variables.
No open issues 🎉
4 resolved since last review
handler-path coverage forCGH.copy(...), but the PR also changes behavior for…errc::kernel_not_supportedis used unqualified here while other SYCL types are qualified…kernel_not_supported, but the message still reads like a…
Review effort: Balanced
uditagarwal97
approved these changes
Nov 3, 2026
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The SYCL
device_globalspec says that if adevice_globalhas thedevice_image_scopeproperty, at least one kernel in its device image must access it. Otherwisecopy/memcpyto or from it must throw anexceptionwitherrc::kernel_not_supported.Based on spec https://github.com/intel/llvm/blob/sycl/sycl/doc/extensions/experimental/sycl_ext_oneapi_device_global.asciidoc
Globals without
device_image_scope, and globals that a kernel does use, are unchanged.Related to: CMPLRLLVM-52861